Skip to content

Make typeck a tcx method which calls typeck_root query#154304

Merged
rust-bors[bot] merged 4 commits intorust-lang:mainfrom
zetanumbers:typeck_root
Mar 29, 2026
Merged

Make typeck a tcx method which calls typeck_root query#154304
rust-bors[bot] merged 4 commits intorust-lang:mainfrom
zetanumbers:typeck_root

Conversation

@zetanumbers
Copy link
Copy Markdown
Contributor

@zetanumbers zetanumbers commented Mar 24, 2026

View all comments

Currently typeck query itself calls tcx.typeck(tcx.typeck_root_def_id(key)) if its key isn't a type-check root. I thought this might be an overhead and made typeck a tcx method which calls typeck_root query instead.

This is a step to simplify cache_on_disk_if query modifier.

@petrochenkov please run perf

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 24, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Mar 24, 2026
[Experiement] Make typeck a tcx method which calls typeck_root query
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 24, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment was marked as resolved.

@rust-log-analyzer

This comment has been minimized.

@zetanumbers
Copy link
Copy Markdown
Contributor Author

r? @petrochenkov

@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Mar 24, 2026
[Experiement] Make typeck a tcx method which calls typeck_root query
@rust-timer

This comment has been minimized.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 24, 2026

☀️ Try build successful (CI)
Build commit: 9e35fa9 (9e35fa9b6e285aa81be6f11080b4412a2f03249e, parent: 0312931d8c0ba1a28268a12c06202b68cbc65f76)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (9e35fa9): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.2% [0.2%, 0.2%] 2
Regressions ❌
(secondary)
0.3% [0.2%, 0.9%] 7
Improvements ✅
(primary)
-1.1% [-6.0%, -0.2%] 38
Improvements ✅
(secondary)
-0.7% [-2.0%, -0.3%] 13
All ❌✅ (primary) -1.0% [-6.0%, 0.2%] 40

Max RSS (memory usage)

Results (primary 3.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.2% [2.1%, 4.3%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.2% [2.1%, 4.3%] 2

Cycles

Results (primary -3.1%, secondary -2.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.1% [-4.0%, -2.3%] 2
Improvements ✅
(secondary)
-2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) -3.1% [-4.0%, -2.3%] 2

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.1% [0.0%, 0.1%] 54
Regressions ❌
(secondary)
0.1% [0.0%, 0.1%] 22
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [0.0%, 0.1%] 54

Bootstrap: 484.31s -> 485.406s (0.23%)
Artifact size: 394.79 MiB -> 396.97 MiB (0.55%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 24, 2026
@zetanumbers zetanumbers marked this pull request as ready for review March 25, 2026 09:15
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 25, 2026
@zetanumbers zetanumbers changed the title [Experiement] Make typeck a tcx method which calls typeck_root query Make typeck a tcx method which calls typeck_root query Mar 25, 2026
@rustbot

This comment has been minimized.

if typeck_root_def_id != def_id {
return tcx.typeck(typeck_root_def_id);
}
assert!(!tcx.is_typeck_child(def_id.to_def_id()));
Copy link
Copy Markdown
Contributor Author

@zetanumbers zetanumbers Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert might be a bit costly due its call to def_kind query.

Copy link
Copy Markdown
Contributor Author

@zetanumbers zetanumbers Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was debug_assert at the time rustc perf was run, so we should probably measure it again. I've changed it to non-debug assert to detect if some compiler dev would call it directly. If it increases compile time, I know how to make a TypeckRoot newtype over LocalDefId as an input to typeck_root to avoid this assertion. Or maybe I'm just too paranoid and it would be fine as debug too.

@fmease
Copy link
Copy Markdown
Member

fmease commented Mar 27, 2026

Average time of auto builds is 3h11 but this one is now at 5h25, clearly runaway / stuck.
@bors retry

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 27, 2026

❗ You can only retry pull requests that are approved and have a previously failed auto build.

Hint: There is currently a pending auto build on this PR. To cancel it, run @bors cancel.

@fmease
Copy link
Copy Markdown
Member

fmease commented Mar 27, 2026

@bors cancel

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 27, 2026

Auto build was cancelled. Cancelled workflows:

The next pull request likely to be tested is #154466.

@rust-bors rust-bors bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 28, 2026
@rust-bors

This comment has been minimized.

@rust-bors rust-bors bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 28, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 28, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@zetanumbers
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 28, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 29, 2026

📌 Commit 1792232 has been approved by petrochenkov

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2026
@rust-bors

This comment has been minimized.

@rust-bors rust-bors bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 29, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 29, 2026

☀️ Test successful - CI
Approved by: petrochenkov
Duration: 3h 11m 25s
Pushing a25435b to main...

@rust-bors rust-bors bot merged commit a25435b into rust-lang:main Mar 29, 2026
12 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing cd14b73 (parent) -> a25435b (this PR)

Test differences

Show 14 test diffs

14 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard a25435bcf7cfc9b953d356eda3a51db8da9e3386 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. pr-check-2: 37m 52s -> 45m 4s (+19.0%)
  2. x86_64-gnu-distcheck: 2h 1m -> 2h 19m (+14.7%)
  3. i686-gnu-nopt-1: 2h 4m -> 2h 22m (+14.5%)
  4. x86_64-gnu-llvm-22-1: 1h 4m -> 1h 14m (+14.4%)
  5. aarch64-apple: 2h 26m -> 2h 46m (+13.8%)
  6. pr-check-1: 28m 18s -> 32m 10s (+13.7%)
  7. i686-gnu-1: 2h 9m -> 2h 26m (+13.5%)
  8. x86_64-msvc-ext3: 1h 44m -> 1h 57m (+12.7%)
  9. x86_64-rust-for-linux: 46m 15s -> 51m 49s (+12.0%)
  10. i686-gnu-2: 1h 32m -> 1h 42m (+11.8%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (a25435b): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.2% [0.2%, 0.2%] 3
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 6
Improvements ✅
(primary)
-1.1% [-6.1%, -0.2%] 41
Improvements ✅
(secondary)
-0.7% [-2.0%, -0.2%] 11
All ❌✅ (primary) -1.0% [-6.1%, 0.2%] 44

Max RSS (memory usage)

Results (primary -2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.4% [-2.4%, -2.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.4% [-2.4%, -2.4%] 1

Cycles

Results (primary -2.9%, secondary -2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.9% [-3.7%, -2.1%] 2
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) -2.9% [-3.7%, -2.1%] 2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 487.525s -> 486.086s (-0.30%)
Artifact size: 394.89 MiB -> 394.81 MiB (-0.02%)

@zetanumbers zetanumbers deleted the typeck_root branch March 30, 2026 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants